-
Notifications
You must be signed in to change notification settings - Fork 51
Allow defining outputs for GHA jobs #845
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
build.sbt
Outdated
| import com.typesafe.tools.mima.core._ | ||
|
|
||
| ThisBuild / tlBaseVersion := "0.8" | ||
| ThisBuild / tlBaseVersion := "0.9" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this be accomplished without bumping the version here? This is effectively a major version change, with all that entails for libraries. I haven't looked closely at whether it's necessary or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried that first but the Mima check failed without the version bump, so I sent another commit with it.
| container: Option[JobContainer] = None, | ||
| environment: Option[JobEnvironment] = None, | ||
| concurrency: Option[Concurrency] = None, | ||
| outputs: Map[String, String] = Map(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the binary-breaking change. To restore binary-compatibility, we need to keep an overload of apply with the old signature, that can delegate to the new one.
It's also a source-breaking change: outputs should be added as the final parameter in the list.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I moved output to the last place, but it's not possible to define overloads with default arguments. What do you recommend?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's not possible to define overloads with default arguments
That's okay: just define the overload with the old signature without default arguments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bincompat required a number of other changes.
Allow defining
outputsfor jobs: https://docs.github.com/en/actions/reference/workflows-and-actions/workflow-syntax#jobsjob_idoutputs.Values are automatically interpreted as expressions, ie, wrapped in
${{ ... }.